Skip to content

Conversation

@calebbrown
Copy link
Contributor

See #1034 for the design doc.

Also adds tests to ensure the new validation rules work as expected.

Signed-off-by: Caleb Brown <[email protected]>
@calebbrown calebbrown marked this pull request as ready for review November 4, 2025 04:14
// There is a direct match, so return the handler immediately.
return handler
}
for suffix, handler := range gitHosts {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess iteration order will be random, if we ever have two suffix patterns that would match.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is true. I have added a comment above to make this clearer.

For now I am okay with this.

return checkPathParts(path, 2)
}

func checkPathParts(path string, partCount int) bool {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comment I guess - maybe count instead of partCount too

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

return true
}

// canonLowerOrg lowercases the first path component in the supplied path.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

canonLowerOrgPath

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed


// validateGitCommtID ensures that candidate is a validly formatted git commit
// ID hash. Git commit IDs are a hex-encoded SHA1 or SHA256 hash.
func validateGitCommtID(candidate string, allowZero bool) error {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

validateGitCommitID throughout

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

@calebbrown calebbrown merged commit 64e59e3 into main Nov 5, 2025
9 checks passed
@calebbrown calebbrown deleted the git_repo_impl branch November 5, 2025 22:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants